Updates to normal and tangent import/export #446

Candoran2 · 2021-05-24T16:30:05Z

@niftools/blender-niftools-addon-reviewer

Overview

Export of normals now respects edges that are marked sharp.
Vertices with different UV coordinates get different tangents on export.
Export now uses Blender tangents instead of pyffi tangent generation.
Nif vertex normals are now imported correctly when dealing with non-normalized normals (i.e. not length 1).
When exporting a skinned mesh that uses partitions, that has faces without partitions, the correct faces are now selected.
The normal map Y channel is now inverted when creating a shader on import.
The selection for collision layer now depends on the game.
A NONE game as the default in the scene tab.
Unresolved texture paths are now stored as they are found in the nif file, meaning they remain functional upon re-export.
Body parts use face maps rather than vertex groups.

Detailed Description

Export of normals previously used vertex normals, of which there is only one per vertex, making sharp edges impossible. Now it uses loop normals, which takes into account edges that are marked sharp.
The pyffi tangent generation that was used created a vertex has that didn't account for differences in UV, so even though vertices had different UV coordinates, if their normals and position were the same, they got the same tangents. With using Blender tangents, this is resolved (though pyffi's update_tangent_space function should still be updated).
Tangents and bitangents are taken from Blender instead of generated. This means that the tangents should now be compatible with normal maps that use MikkTSpace. Note that it does not mean shading will agree completely, since Blender calculates the binormal per-pixel rather than per vertex, like is done in-game. xNormal is a good option for generating normal maps that calculate the binormal per-vertex and uses MikkTSpace. The tangents in the nif file are the negative of the Blender bitangents, and the nif bitangents are the Blender tangents. This is because in a nif, the tangents point in the direction of positive V, and the bitangents towards positive U. Moreover, the Nif V coordinates are calculated from Blender via 1 - V, so positive is in the opposite direction.
When normals were imported that were not length 1, Blender normalized them itself. However, this gave different results for different loops. Hence, edges which were actually smooth became marked sharp, and vertices were split upon export. The new code pre-normalizes the normals before setting them, leaving smooth edges intact.
For various games, exported skinned meshes require every triangle to be marked with at least one partition. If they weren't, the exporter was supposed to select the triangles that lacked a partition and error. Because we are operating on a triangulated copy of the mesh, and not the mesh itself, the selected faces didn't show. Now, the triangles without partitions are mapped back to the polygons in the original mesh, and are selected as they should be.
Most (all?) games invert the Y (green) channel of the normal map compared to blender. Shader import now reflects this, and re-uses the invertY group node if available.
The selectable collision layers under a rigid body now depend on the selected game, meaning that Skyrim and Fallout 3 collision layers are available. However, the import/export is still not fully functional for those games because of the outdated nif.xml.
There is now a NONE option among the games in the scene tab. This is the default. This way, we can error immediately and let the user know what they should do.
Skyrim and Fallout3 body parts used to be stored as vertex groups. One problem with this is that those partitions (or at least those generated by the update_skin_partition function) only allow for one partition per triangle, whereas vertex groups can overlap. Moreover, individual vertices can be assigned rather than polygons. To solve this, they are now stored as face maps. Blender face maps only allow one face map assignment per polygon, and are more in-line with the per-polygon assignment structure already used by the export.

Fixes Known Issues

#444
#449

Documentation

Added a design decisions document, and a section on why tangents are exported the way they are (using Blender's MikkTSpace and flipping the tangents/bitangents).

Testing

[Overview of testing required to ensure functionality is correctly implemented]

Manual

[Set of steps to manually verify updates are working correctly]

Automated

[List of tests run, updated or added to avoid future regressions]

Additional Information

[Anything else you deem relevant]

… mesh (rather than the copy with tris).

…ed over update_tangent_space from pyffi and changed it so that vertices with same normals and position, but different UV coordinated no longer have the same tangents (because tangent calculation depends on UV coordinate)

…ating them separately.

… can't be, due to how the nif format works) when the normals weren't normalized, caused by Blender normalization being inconsistent.

…e only once per vertex, instead of first copying to all loops and then normalizing it independently multiple times.

neomonkeus · 2021-05-26T18:15:38Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

+
+            use_tangents = False
+            if mesh_uv_layers and mesh_hasnormals:
+                if bpy.context.scene.niftools_scene.game in ('OBLIVION', 'FALLOUT_3', 'SKYRIM') or (bpy.context.scene.niftools_scene.game in self.texture_helper.USED_EXTRA_SHADER_TEXTURES):


Can extract out game to variable

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

neomonkeus · 2021-05-26T18:21:25Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py


    def select_unassigned_polygons(self, b_mesh, b_obj, polygons_without_bodypart):
        """Select any faces which are not weighted to a vertex group"""
+        ngon_mesh = b_obj.data
+        # make vertex: poly map of the untriangulated mesh
+        vert_poly_dict = dict(zip(range(len(ngon_mesh.vertices)), [set() for i in range(len(ngon_mesh.vertices))]))


len(ngon_mesh.vertices) used twice so would pull out to num_vertices but will also improve readability.

Simplify this to vert_poly_dict = {i: set() for for i in range(len(ngon_mesh.vertices))}

neomonkeus · 2021-05-26T18:23:19Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

+        if (len(tangents) != nr_vertices) or (len(bitangents) != nr_vertices):
+            raise NifError(f'Number of tangents or bitangents does not agree with number of vertices in {trishape.name}')
+
+        if as_extra:


use_extra_data

simplify as
if not (len(verts) == len(tangents) == len(bitangents)):

neomonkeus · 2021-05-26T18:23:42Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

@@ -644,3 +691,46 @@ def ensure_tri_modifier(self, b_obj):
                break
        else:
            b_obj.modifiers.new('Triangulate', 'TRIANGULATE')
+
+    def add_defined_tangents(self, trishape, tangents, bitangents, as_extra):


use_extra_data?

What do you mean by this?

neomonkeus · 2021-05-26T18:24:34Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

+
+    def add_defined_tangents(self, trishape, tangents, bitangents, as_extra):
+        # check if size of tangents and bitangents is equal to num_vertices
+        nr_vertices = trishape.data.num_vertices


nr > num?

…amed nr_vertices to num_vertices and renamed argument as_extra to as_extra_data.

HENDRIX-ZT2

test

HENDRIX-ZT2 · 2021-05-26T19:39:37Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

@@ -77,6 +79,7 @@ def export_tri_shapes(self, b_obj, n_parent, trishape_name=None):

        # get mesh from b_obj
        b_mesh = self.get_triangulated_mesh(b_obj)
+        b_mesh.calc_normals_split()


I use me.calc_tangents() in my plugins, generates everything we need.

I use calc_tangents() later, too, but only if the tangents need to be exported.

I see. Not sure if blender avoids recalculating the loop normals data in this case.

Not sure. I tried it on a simple subdivided cube with 49000 vertices (close to the nif limit of ~65000 vertices). Calc_normals_split was still too fast to measure (just displayed as 0.0 s) whereas calc_tangents() was approximately 0.05, regardless of whether calc_normals_split was called before or not. Given that calc_normals_split() is so fast, it's not really possible to check by timing whether calc_tangents does it again (but then it doesn't really matter whether it does). Based on the code of rna_Mesh_calc_tangents(), I'd wager it does avoid recalculating.

Either way, since calc_normals_split() seems to be so much faster, I reckon it'd be worth not calling calc_tangents() in some cases even if that did cause calc_normals_split() to be called twice in others.

Seems like the times are pretty good, although I don't know how beefy your hardware is.
Happy enough for now as we are still in the "I want this feature", rather than "this feature is taking to long to run" phase.

I am sure there are plenty of bottleknecks which I suppose NumPy may end up solving in the long run.

HENDRIX-ZT2 · 2021-05-26T19:41:51Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

@@ -265,6 +279,9 @@ def export_tri_shapes(self, b_obj, n_parent, trishape_name=None):
                        vertlist.append(vertquad[0])
                        if mesh_hasnormals:
                            normlist.append(vertquad[2])
+                        if use_tangents:
+                            tanlist.append(b_mesh.loops[loop_index].tangent)


tanlist, normlist... I would give neater names or use underscores.
Or just go
coords
normals
tangents

also, if we use tangents we need to have normals so we should perhaps nest the if?

Tanlist and bitlist names were taken from the other lists that already existed (vertlist, normlist, vcollist, uvlist, trilist). If you want to change that naming scheme, it can certainly be done, but I think that might be better packaged up in a future PR.

I thought since you're adding new lists you might as well fix the naming scheme there. ;)

Either or, I know my dictionary checker will like having the underscores or better names.
Don't mind if done as part of this or subsequent PR.

HENDRIX-ZT2 · 2021-05-26T19:45:50Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

+        if (len(tangents) != nr_vertices) or (len(bitangents) != nr_vertices):
+            raise NifError(f'Number of tangents or bitangents does not agree with number of vertices in {trishape.name}')
+
+        if as_extra:


simplify as
if not (len(verts) == len(tangents) == len(bitangents)):

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

io_scene_niftools/modules/nif_import/geometry/vertex/__init__.py

HENDRIX-ZT2 · 2021-05-26T20:00:51Z

io_scene_niftools/modules/nif_export/geometry/mesh/__init__.py

+                # B_tan: +d(B_u), B_bit: +d(B_v) and N_tan: +d(N_v), N_bit: +d(N_u)
+                # moreover, N_v = 1 - B_v, so d(B_v) = - d(N_v), therefore N_tan = -B_bit and N_bit = B_tan
+                self.add_defined_tangents(trishape,
+                                          tangents=-bitlist,


This is confusing. So you're exporting blender's bitangents as nif tangents? Seems like axis correction weirdness.

Do we get a performance gain from calculating the bitangents with numpy over crossing blender vectors?

Correct, it is axis correction weirdness. In Nifs, tangents point to the positive (Nif) V direction and bitangents point to the positive (Nif) U direction (hence N_tan: +d(N_v), N_bit: +d(N_u)), while Blender tangents point in the positive U direction, and bitangents in the positive V direction. That's why they're flipped (tangents become bitangents and vice versa).

Then, the Nif V coordinate = 1 - Blender V coordinate, so the direction of positive V is flipped (hence the minus sign).
I tried to leave that information in the comments as concisely as possible, but maybe it should be in the technical documentation somewhere, too.

As for the speed of calculating the bitangents with numpy vs individually, I haven't tested the alternative. However, one of numpy's big strengths is array operations, so I would be extremely surprised if it were any slower than calculating them individually.

This is confusing. So you're exporting blender's bitangents as nif tangents? Seems like axis correction weirdness.

Do we get a performance gain from calculating the bitangents with numpy over crossing blender vectors?

Where would be the best place in the documentation to outline why tangent space is calculated from Blender the way that it is? I would assume it doesn't need to be in user, but there doesn't really seem to be a good place in it in development.

We can create a design decisions section for anything that we feel needs enough of an explaination about it.

…t in export/geometry/mesh. Pulled normalization of imported normals out to separate function. Use ndarray.asbytes() instead of struct.pack() for conversion to binary data.

…s, so that they will use title text without needing custim link text. Updated spelling of vertex colour to american for consistency.

…ported the way they are.

…n-game.

…Oblivion collision layers. No Skyrim functionality - best fix for that would be pyffi/nif.xml update.

…or when they forget to set the game.

… if file is not found, allowing re-export with working textures.

…created again if it already existed.

…ace maps.

Candoran2 added 5 commits May 24, 2021 17:26

Changed select_unassigned_polygons to actually select in the original…

cd3b0ef

… mesh (rather than the copy with tris).

Added export of (bi)tangents directly from Blender, rather than gener…

f208d40

…ating them separately.

Fixed issue with edges being marked sharp on import (even though they…

afd108e

… can't be, due to how the nif format works) when the normals weren't normalized, caused by Blender normalization being inconsistent.

Refactored so that the to_tuple() conversion and normalization is don…

9f6bd87

…e only once per vertex, instead of first copying to all loops and then normalizing it independently multiple times.

neomonkeus reviewed May 26, 2021

View reviewed changes

Updated code as per PR review. Pulled game out to local variable, ren…

9a2872a

…amed nr_vertices to num_vertices and renamed argument as_extra to as_extra_data.

HENDRIX-ZT2 reviewed May 26, 2021

View reviewed changes

Candoran2 added 5 commits May 27, 2021 20:10

Implemented PR review comments from Hendrix: Renamed variables xxxlis…

c9a5e93

…t in export/geometry/mesh. Pulled normalization of imported normals out to separate function. Use ndarray.asbytes() instead of struct.pack() for conversion to binary data.

Updated documentation for export. Moved custom anchors to above title…

c4d36d3

…s, so that they will use title text without needing custim link text. Updated spelling of vertex colour to american for consistency.

Added Design Decisions document. Added section on why tangents are ex…

3303b74

…ported the way they are.

Fixed mistype leading to syntax error.

6a96a76

Implemented inversion of normal map Y channel to display normals as i…

e3fa242

…n-game.

neomonkeus approved these changes Jul 1, 2021

View reviewed changes

Candoran2 added 7 commits July 8, 2021 00:17

Changed collision layer to be game dependent, rather than relying on …

8018ff6

…Oblivion collision layers. No Skyrim functionality - best fix for that would be pyffi/nif.xml update.

Added default NONE game for in the scene tab, and a comprehensive err…

0848906

…or when they forget to set the game.

Added game set check to kf export (previously only in nif export).

58c2054

Changed texture import so that original nif texture path is preserved…

7abfce9

… if file is not found, allowing re-export with working textures.

Updated normal map Y-invert code so that the node group does not get …

f53bc8f

…created again if it already existed.

Removed obsolete part flag storage.

16936f2

Converted body partitions from using Blender vertex groups to using f…

c7f6ff2

…ace maps.

neomonkeus approved these changes Sep 25, 2021

View reviewed changes

neomonkeus merged commit 92c9afc into niftools:develop Sep 25, 2021

This was referenced Sep 25, 2021

Release/v0.0.9 #459

Merged

Release/v0.0.9 #461

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to normal and tangent import/export #446

Updates to normal and tangent import/export #446

Candoran2 commented May 24, 2021 •

edited

Loading

neomonkeus May 26, 2021 •

edited

Loading

neomonkeus May 26, 2021

HENDRIX-ZT2 May 27, 2021

neomonkeus May 26, 2021

HENDRIX-ZT2 May 26, 2021

neomonkeus May 26, 2021

Candoran2 May 26, 2021

neomonkeus May 26, 2021

HENDRIX-ZT2 May 26, 2021

HENDRIX-ZT2 left a comment

HENDRIX-ZT2 May 26, 2021

Candoran2 May 26, 2021

HENDRIX-ZT2 May 26, 2021

Candoran2 May 27, 2021 •

edited

Loading

neomonkeus May 27, 2021 •

edited

Loading

HENDRIX-ZT2 May 26, 2021

HENDRIX-ZT2 May 26, 2021

Candoran2 May 26, 2021

HENDRIX-ZT2 May 26, 2021

neomonkeus May 27, 2021

HENDRIX-ZT2 May 26, 2021

HENDRIX-ZT2 May 26, 2021

Candoran2 May 26, 2021

Candoran2 Jun 1, 2021

neomonkeus Jun 1, 2021

Updates to normal and tangent import/export #446

Updates to normal and tangent import/export #446

Conversation

Candoran2 commented May 24, 2021 • edited Loading

Overview

Detailed Description

Fixes Known Issues

Documentation

Testing

Manual

Automated

Additional Information

neomonkeus May 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HENDRIX-ZT2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Candoran2 May 27, 2021 • edited Loading

Choose a reason for hiding this comment

neomonkeus May 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Candoran2 commented May 24, 2021 •

edited

Loading

neomonkeus May 26, 2021 •

edited

Loading

Candoran2 May 27, 2021 •

edited

Loading

neomonkeus May 27, 2021 •

edited

Loading